Skip to content

refactor(llm): remove LangChain imports from core modules#1770

Open
Pouyanpi wants to merge 2 commits intofeat/langchain-decouple/stack-4-rename-asyncfrom
feat/langchain-decouple/stack-5-remove-core-imports
Open

refactor(llm): remove LangChain imports from core modules#1770
Pouyanpi wants to merge 2 commits intofeat/langchain-decouple/stack-4-rename-asyncfrom
feat/langchain-decouple/stack-5-remove-core-imports

Conversation

@Pouyanpi
Copy link
Copy Markdown
Collaborator

@Pouyanpi Pouyanpi commented Apr 7, 2026

Part of the LangChain decoupling stack:

  1. stack-1: canonical types (feat(types): add framework-agnostic LLM type system #1745)
  2. stack-2: adapter and framework registry (feat(llm): add LangChain adapter and framework registry #1759)
  3. stack-3: pipeline rewrite + caller migration (refactor(llm)!: atomic switch to LLMModel protocol #1760)
  4. stack-4: rename generate/stream to generate_async/stream_async (refactor(llm): rename generate/stream to generate_async/stream_async #1769)
  5. stack-5: remove LangChain imports from core modules (this PR)
  6. stack-6: move LangChain implementations into integrations/langchain/ (refactor(llm): move LangChain implementations into integrations/langchain/ #1772)
  7. stack-7: framework-owned provider registry (refactor(llm): framework-owned provider registry #1773)
  8. stack-8: framework-agnostic test infrastructure (TODO)

Description

  • Remove PromptTemplate from hallucination actions (was a no-op)
  • Replace Runnable isinstance check with duck-type ainvoke check in action_dispatcher
  • Remove PromptTemplate and llm.bind().invoke() from evaluate_factcheck, use LLMModel.generate_async via event loop
  • Make eval CLI LangChain cache a lazy optional import

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 7, 2026

Greptile Summary

This PR removes direct LangChain imports from four core modules as part of the framework-decoupling stack. The changes are: duck-typing ainvoke in action_dispatcher instead of isinstance(Runnable), lazy-loading the LangChain SQLite cache in the eval CLI, replacing llm.bind().invoke() with self.llm.generate_async() in the factcheck evaluator, and dropping a no-op PromptTemplate import from the hallucination actions.

Confidence Score: 5/5

  • Safe to merge; the only new finding is a P2 note about the broadened duck-type guard, and prior P1 concerns in evaluate_factcheck.py were already raised in earlier review threads.
  • No new P0 or P1 issues introduced by this PR. The duck-type ainvoke change has a broader match than the old isinstance(Runnable) check, but mis-routed calls are caught by the existing broad exception handler. The two significant concerns in evaluate_factcheck.py (async-pattern inconsistency and observability bypass) were already flagged in previous threads. All remaining findings are P2.
  • nemoguardrails/evaluate/evaluate_factcheck.py — previously-flagged async-pattern and observability issues remain open.

Important Files Changed

Filename Overview
nemoguardrails/actions/action_dispatcher.py Replaced isinstance(fn, Runnable) with duck-type hasattr(fn, "ainvoke") and callable(fn.ainvoke) — removes the langchain_core import but broadens the match to any object with a callable ainvoke, which could silently mis-route custom action classes that have ainvoke but don't follow LangChain's input=params calling convention.
nemoguardrails/eval/cli.py LangChain cache imports moved to a lazy try/except block inside check_compliance, making the module importable without LangChain installed. A follow-up fix in this same PR corrects the log order so the success message is only printed after the cache is successfully configured.
nemoguardrails/evaluate/evaluate_factcheck.py Replaced LangChain-based LLM calls with self.llm.generate_async() — removes langchain dependency but introduces an inconsistent async pattern (get_or_create_event_loop().run_until_complete() in create_negative_samples vs asyncio.run() in check_facts) and bypasses the llm_call observability wrapper already present elsewhere in this module (both previously noted).
nemoguardrails/library/hallucination/actions.py Removed the no-op PromptTemplate import; remaining logic is unchanged and uses llm_call / asyncio.gather correctly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[execute_action called] --> B{action registered?}
    B -- No --> Z[return None, failed]
    B -- Yes --> C{isclass?}
    C -- Yes --> D[instantiate lazy]
    C -- No --> E{isfunction or ismethod?}
    D --> E
    E -- Yes --> F[call fn directly]
    F --> G{iscoroutine?}
    G -- Yes --> H[await result]
    G -- No --> I[log sync warning, use result]
    E -- No --> J{"hasattr(ainvoke) and callable? (duck-type for Runnable)"}
    J -- Yes --> K["await fn.ainvoke(input=params)"]
    J -- No --> L{has run method?}
    L -- No --> M[raise Exception]
    L -- Yes --> N["call fn.run(**params)"]
    H --> O[return result, success]
    I --> O
    K --> O
    N --> O
    O --> P{LLMCallException?}
    P -- Yes --> Q[re-raise]
    P -- No --> R[log warning, return None, failed]
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: nemoguardrails/actions/action_dispatcher.py
Line: 219-222

Comment:
**Duck-type check is broader than the original `Runnable` guard**

The old `isinstance(fn, Runnable)` only matched LangChain `Runnable` objects. The new check matches *any* registered action that has a callable `ainvoke` attribute, and then unconditionally calls it as `fn.ainvoke(input=params)` — the LangChain keyword argument convention. A custom action class that exposes `ainvoke` with a different signature (e.g. positional args, or no `input=` key) will never reach the `run()` fallback; it will raise a `TypeError` that is swallowed by the broad `except Exception` handler, returning `"failed"` silently.

If the intent is to keep this path strictly for LangChain `Runnable`-compatible objects, consider tightening the guard or documenting the expected calling convention in the comment:

```suggestion
                    elif hasattr(fn, "ainvoke") and callable(fn.ainvoke):  # type: ignore[union-attr]
                        # Duck-type check for objects that follow the LangChain Runnable
                        # protocol (ainvoke(input=...) signature) without importing langchain.
                        # Custom actions with a differently-signed ainvoke should use 'run' instead.
                        result = await fn.ainvoke(input=params)  # type: ignore[union-attr]
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (6): Last reviewed commit: "fix(eval): log cache status after enabli..." | Re-trigger Greptile

Comment thread nemoguardrails/evaluate/evaluate_factcheck.py
Comment thread nemoguardrails/evaluate/evaluate_factcheck.py
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-4-rename-async branch from b9bb707 to 41c9073 Compare April 7, 2026 16:24
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-5-remove-core-imports branch from 3807a9a to 77864fa Compare April 7, 2026 16:24
@Pouyanpi Pouyanpi self-assigned this Apr 7, 2026
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-5-remove-core-imports branch from 77864fa to f2a54ce Compare April 7, 2026 16:29
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2026

Codecov Report

❌ Patch coverage is 20.00000% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
nemoguardrails/eval/cli.py 0.00% 7 Missing ⚠️
nemoguardrails/evaluate/evaluate_factcheck.py 20.00% 4 Missing ⚠️
nemoguardrails/actions/action_dispatcher.py 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Collaborator

@tgasser-nv tgasser-nv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a minor logging nit

Comment thread nemoguardrails/eval/cli.py Outdated
- Remove PromptTemplate from hallucination actions (was a no-op)
- Replace Runnable isinstance check with duck-type ainvoke check
  in action_dispatcher
- Remove PromptTemplate and llm.bind().invoke() from evaluate_factcheck,
  use LLMModel.generate_async via event loop
- Make eval CLI LangChain cache a lazy optional import
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-4-rename-async branch from 0991b48 to 61abe37 Compare April 14, 2026 10:13
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-5-remove-core-imports branch from 514620d to 5ed106b Compare April 14, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants